From f4b0322885670128c9d37f6e627a627c2b66b6d6 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 9 Jul 2025 18:04:08 -0400 Subject: [PATCH] repo: Add new API to write config with reload+validation The `ostree config set` CLI should really disallow writing invalid values. To implement that, add a new API that also *reloads* to the new config, and rolls back on failure. Closes: https://github.com/ostreedev/ostree/issues/1827 Signed-off-by: Colin Walters --- apidoc/ostree-sections.txt | 1 + src/libostree/libostree-devel.sym | 1 + src/libostree/ostree-repo.c | 79 +++++++++++++++++++++++-------- src/libostree/ostree-repo.h | 3 ++ src/ostree/ot-builtin-config.c | 2 +- tests/test-config.sh | 19 +++++++- 6 files changed, 84 insertions(+), 21 deletions(-) diff --git a/apidoc/ostree-sections.txt b/apidoc/ostree-sections.txt index a43d1079..e392f1e0 100644 --- a/apidoc/ostree-sections.txt +++ b/apidoc/ostree-sections.txt @@ -352,6 +352,7 @@ ostree_repo_get_remote_list_option ostree_repo_get_remote_option ostree_repo_get_parent ostree_repo_write_config +ostree_repo_write_config_and_reload OstreeRepoTransactionStats ostree_repo_scan_hardlinks ostree_repo_prepare_transaction diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 610a36b7..f7e7eb8b 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -32,6 +32,7 @@ global: LIBOSTREE_2025.3 { global: + ostree_repo_write_config_and_reload; ostree_deployment_is_soft_reboot_target; ostree_sysroot_deployment_can_soft_reboot; ostree_sysroot_deployment_set_soft_reboot; diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 4e2f47d0..9aec76f4 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -435,6 +435,7 @@ pop_repo_lock (OstreeRepo *self, OstreeRepoLockType lock_type, gboolean blocking return TRUE; } +static gboolean reload_config_inner (OstreeRepo *self, GCancellable *cancellable, GError **error); /** * ostree_repo_lock_push: @@ -1566,6 +1567,9 @@ ostree_repo_copy_config (OstreeRepo *self) * @error: a #GError * * Save @new_config in place of this repository's config file. + * + * Note: This will not validate many elements of the configuration. + * Prefer `ostree_repo_write_config_and_reload`. */ gboolean ostree_repo_write_config (OstreeRepo *self, GKeyFile *new_config, GError **error) @@ -1618,6 +1622,36 @@ ostree_repo_write_config (OstreeRepo *self, GKeyFile *new_config, GError **error return TRUE; } +/** + * ostree_repo_write_config_and_reload: + * @self: Repo + * @new_config: Overwrite the config file with this data, and reload + * @error: a #GError + * + * Save @new_config in place of this repository's config file and reload. + * The config will be validated. + */ +gboolean +ostree_repo_write_config_and_reload (OstreeRepo *self, GKeyFile *new_config, GError **error) +{ + g_return_val_if_fail (self->inited, FALSE); + + g_autoptr (GKeyFile) old_config = g_steal_pointer (&self->config); + // Test reloading with the new config + self->config = new_config; + gboolean r = reload_config_inner (self, NULL, error); + self->config = g_steal_pointer (&old_config); + if (!r) + { + // Best effort to revert back to the old config, but if that fails + // we're in a doubly bad state. + (void)reload_config_inner (self, NULL, NULL); + return FALSE; + } + // Now perform the actual write + return ostree_repo_write_config (self, new_config, error); +} + /* Bind a subset of an a{sv} to options in a given GKeyfile section */ static void keyfile_set_from_vardict (GKeyFile *keyfile, const char *section, GVariant *vardict) @@ -2993,19 +3027,6 @@ reload_core_config (OstreeRepo *self, GCancellable *cancellable, GError **error) g_autofree char *contents = NULL; g_autofree char *parent_repo_path = NULL; gboolean is_archive; - gsize len; - - g_clear_pointer (&self->config, g_key_file_unref); - self->config = g_key_file_new (); - - contents = glnx_file_get_contents_utf8_at (self->repo_dir_fd, "config", &len, NULL, error); - if (!contents) - return FALSE; - if (!g_key_file_load_from_data (self->config, contents, len, 0, error)) - { - g_prefix_error (error, "Couldn't parse config file: "); - return FALSE; - } version = g_key_file_get_value (self->config, "core", "repo_version", error); if (!version) @@ -3353,6 +3374,18 @@ reload_sysroot_config (OstreeRepo *self, GCancellable *cancellable, GError **err return TRUE; } +static gboolean +reload_config_inner (OstreeRepo *self, GCancellable *cancellable, GError **error) +{ + if (!reload_core_config (self, cancellable, error)) + return FALSE; + if (!reload_remote_config (self, cancellable, error)) + return FALSE; + if (!reload_sysroot_config (self, cancellable, error)) + return FALSE; + return TRUE; +} + /** * ostree_repo_reload_config: * @self: repo @@ -3367,13 +3400,21 @@ reload_sysroot_config (OstreeRepo *self, GCancellable *cancellable, GError **err gboolean ostree_repo_reload_config (OstreeRepo *self, GCancellable *cancellable, GError **error) { - if (!reload_core_config (self, cancellable, error)) - return FALSE; - if (!reload_remote_config (self, cancellable, error)) - return FALSE; - if (!reload_sysroot_config (self, cancellable, error)) + g_clear_pointer (&self->config, g_key_file_unref); + self->config = g_key_file_new (); + + gsize len; + g_autofree char *contents + = glnx_file_get_contents_utf8_at (self->repo_dir_fd, "config", &len, NULL, error); + if (!contents) return FALSE; - return TRUE; + if (!g_key_file_load_from_data (self->config, contents, len, 0, error)) + { + g_prefix_error (error, "Couldn't parse config file: "); + return FALSE; + } + + return reload_config_inner (self, cancellable, error); } gboolean diff --git a/src/libostree/ostree-repo.h b/src/libostree/ostree-repo.h index d38fad9a..460188b0 100644 --- a/src/libostree/ostree-repo.h +++ b/src/libostree/ostree-repo.h @@ -192,6 +192,9 @@ OstreeRepo *ostree_repo_get_parent (OstreeRepo *self); _OSTREE_PUBLIC gboolean ostree_repo_write_config (OstreeRepo *self, GKeyFile *new_config, GError **error); +_OSTREE_PUBLIC +gboolean ostree_repo_write_config_and_reload (OstreeRepo *self, GKeyFile *new_config, + GError **error); /** * OstreeRepoCommitState: diff --git a/src/ostree/ot-builtin-config.c b/src/ostree/ot-builtin-config.c index 039656c5..c269a40e 100644 --- a/src/ostree/ot-builtin-config.c +++ b/src/ostree/ot-builtin-config.c @@ -116,7 +116,7 @@ ostree_builtin_config (int argc, char **argv, OstreeCommandInvocation *invocatio config = ostree_repo_copy_config (repo); g_key_file_set_string (config, section, key, value); - if (!ostree_repo_write_config (repo, config, error)) + if (!ostree_repo_write_config_and_reload (repo, config, error)) return FALSE; } else if (!strcmp (op, "get")) diff --git a/tests/test-config.sh b/tests/test-config.sh index 2d9aaf53..d4a35b4c 100755 --- a/tests/test-config.sh +++ b/tests/test-config.sh @@ -21,7 +21,7 @@ set -euo pipefail . $(dirname $0)/libtest.sh -echo '1..3' +echo '1..4' ostree_repo_init repo ${CMD_PREFIX} ostree remote add --repo=repo --set=xa.title=Flathub --set=xa.title-is-set=true flathub https://dl.flathub.org/repo/ @@ -97,3 +97,20 @@ if ${CMD_PREFIX} ostree config --repo=repo unset core.lock-timeout-secs extra 2> fi assert_file_has_content err.txt "error: Too many arguments given" echo "ok config unset" + +# Test config validation - should reject invalid values immediately +if ${CMD_PREFIX} ostree config --repo=repo set core.min-free-space-size "invalidvalue" 2>err.txt; then + assert_not_reached "ostree config set should reject invalid core.min-free-space-size" +fi +assert_file_has_content err.txt "Invalid min-free-space-size" + +# Set a valid value, now try resetting it +ostree config --repo=repo set core.min-free-space-size "100MB" +if ${CMD_PREFIX} ostree config --repo=repo set core.min-free-space-size "invalidvalue" 2>err.txt; then + assert_not_reached "ostree config set should reject invalid core.min-free-space-size" +fi +assert_file_has_content err.txt "Invalid min-free-space-size" +v=$(${CMD_PREFIX} ostree config --repo=repo get core.min-free-space-size) +assert_streq "$v" "100MB" + +echo "ok config validation" -- 2.30.2